feat(decisions): surface L1 supersession lineage in the archive#1416
Conversation
The supersede backend (#1339) records a parent_decision_id chain and serves it at GET /api/decisions/{id}/history, but the app only showed a flat 'Superseded' label with no way to see what replaced what. Add an expandable history trail on answered cards that have a parent (a revision): it lazily fetches the chain on first expand (oldest first) and renders each step's question + chosen answer + when. Originals with no parent show no affordance. Read-only, reuses the shell tokens already in the file; vitest covers the expand/fetch/render path and the no-parent case. Visual check against the live app deferred to a live session (same as the canvas read-only slices).
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 2 minutes and 26 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| async function toggle() { | ||
| if (open) { | ||
| setOpen(false); | ||
| return; | ||
| } | ||
| setOpen(true); | ||
| if (chain || loading) return; | ||
| setLoading(true); | ||
| setError(null); | ||
| try { | ||
| const res = await fetch(`/api/decisions/${decisionId}/history`); | ||
| if (!res.ok) throw new Error("Could not load history."); | ||
| setChain(asDecisionList(await res.json())); | ||
| } catch (e) { | ||
| setError(e instanceof Error ? e.message : "Could not load history."); |
There was a problem hiding this comment.
💡 Bug: HistoryTrail can setState after unmount during fetch
toggle() issues an async fetch and then calls setLoading, setChain, and setError in the try/catch/finally. If the card unmounts while the request is in flight (e.g. the answered list refreshes via the parent's load() polling, or the user switches tabs), these setState calls run on an unmounted component. There is no AbortController and no mounted-ref guard, so the in-flight request is neither cancelled nor ignored. Impact is limited to a wasted request and a possible React state-update-on-unmounted-component warning rather than a crash, hence minor. Suggested fix: track mounted state with a ref (or use an AbortController stored per-toggle) and bail out before setting state.
Guard state updates with a mounted ref:
const mounted = useRef(true);
useEffect(() => () => { mounted.current = false; }, []);
// ...inside toggle(), wrap each post-await setState:
if (!mounted.current) return;
setChain(asDecisionList(await res.json()));
Was this helpful? React with 👍 / 👎
|
Note Your trial team has used its Gitar budget, so automatic reviews are paused. Upgrade now to unlock full capacity. Comment "Gitar review" to trigger a review manually. Code Review 👍 Approved with suggestions 0 resolved / 1 findingsImplements an expandable lineage trail for superseded decisions, enabling users to view revision history. Ensure the 💡 Bug: HistoryTrail can setState after unmount during fetch📄 desktop/src/apps/DecisionsApp.tsx:337-351
Guard state updates with a mounted ref🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Important Your trial ends in 3 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more. Was this helpful? React with 👍 / 👎 | Gitar |
| if (chain || loading) return; | ||
| setLoading(true); | ||
| setError(null); | ||
| try { |
There was a problem hiding this comment.
SUGGESTION: No abort handling on the in-flight history fetch. If the user collapses the panel, switches tabs, or the card unmounts (e.g. silent refresh re-renders the archive after an answer in the same view) before the request resolves, setLoading/setChain/setError will still run on an unmounted component. Consider an AbortController per fetch and check controller.signal.aborted (or a mounted ref) before calling the setters in the finally block.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| fireEvent.click(historyBtn); | ||
| await flush(); | ||
|
|
||
| // The chain is fetched lazily on expand and rendered oldest first. |
There was a problem hiding this comment.
WARNING: The PR description and the inline comment both explicitly claim the chain is "rendered oldest first", but this test never asserts ordering. The mock returns [original, revision] in the right order and the assertion is only expect(screen.getByText(/superseded/i)).toBeTruthy(), which would pass even if the component rendered newest-first or in reverse. Add an ordering assertion (e.g. read the <li> nodes and verify the original's question appears before the revision's) so this behaviour is actually pinned by the test suite.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Both issues from the previous review are resolved in commit
No new issues found in the incremental changes. Files Reviewed (2 files)
Previous Review Summary (commit b3bbc78)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit b3bbc78)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICALNone WARNING
SUGGESTION
Files Reviewed (2 files)
Reviewed by minimax-m3 · Input: 39.2K · Output: 1.3K · Cached: 55.5K |
Fold #1416 review: an archive card can unmount mid-fetch (tab switch or list refresh) while /history is in flight, so the post-await setState would warn and leak. Track liveness with a ref cleared on unmount and gate every post-await setState on it (gitar Bug; kilo concurs). Also assert the oldest-first ordering the description claims, via a testid on the history list (kilo).
What
The Decisions L1 supersede backend (#1339) records a
parent_decision_idchain and serves it atGET /api/decisions/{id}/history, but the app only rendered a flatSupersededlabel with no way to see the lineage. This adds an expandable history trail.Behaviour
View historytoggle./api/decisions/{id}/history(one request per card, only when opened) and renders the chain oldest first: each step's question, chosen answer (orsuperseded), and relative time.Tests
DecisionsApp.test.tsx: 2 new cases (no affordance for a parentless decision; lazy-load + render the lineage for a revision) plus the existing 5. All 7 green;tsc -bclean.Verification note
Behaviourally verified via vitest (jsdom). Visual check against the running app is deferred to a live session, since the trail needs the live backend to populate, consistent with the canvas read-only slices.
Completes Decisions MVP item 6 (L1 supersede) on the frontend; the backend already shipped.